Skip to content

🐛 fix: Group Scope Picker and Name Resolution Past 200 Groups - #115

Open
dustinhealy wants to merge 7 commits into
mainfrom
fix/issue-96-group-scope-pagination
Open

🐛 fix: Group Scope Picker and Name Resolution Past 200 Groups#115
dustinhealy wants to merge 7 commits into
mainfrom
fix/issue-96-group-scope-pagination

Conversation

@dustinhealy

@dustinhealy dustinhealy commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Deployments with more groups than the backend's 200-per-request cap (e.g. 1500+ Entra/AD groups synced via SSO) could not manage group-based configuration: the scope picker only ever loaded the first 200 groups, and existing group scopes outside that window displayed raw Mongo ObjectIds instead of group names.

The picker's create view now searches groups server side through a new useGroupSearch hook (debounced search plus offset pagination built on the existing getGroupsFn search/limit/offset support, mirroring the GroupsTab pattern), so every group is reachable by search or paging regardless of total count. Search strings are clamped to the backend's 200-character limit, which otherwise rejects the request with a 400. Groups that already have a configuration are no longer filtered out after pagination; they render as disabled entries with an "Already configured" badge, so page contents always match the server-derived page count and eligible groups on later pages stay reachable.

getAvailableScopesFn no longer derives its name map from the first page of the groups list. It resolves names for exactly the group principalIds that have config overrides via GET /api/admin/groups/:id, deduplicated and fetched in small parallel batches, falling back to the principalId for groups that cannot be fetched (e.g. deleted groups), so existing scopes always show their real names.

Fixes #96

Change Type

  • Bug fix (non-breaking change which fixes an issue)

Testing

New unit tests: scopes.names.test.ts reproduces the ObjectId fallback against a simulated 1500-group backend (fails on main, passes here), useGroupSearch.test.tsx exercises the debounced search, page-offset, search-clamping, and enabled-gating behavior with only the HTTP layer mocked, and ScopeSelector.test.tsx covers the truthful-pagination regression (a fully configured first page renders disabled entries with working pagination to an eligible page two, failing against the previous filtering). Local gates: full vitest suite passes (812 tests), eslint --max-warnings 0 clean, tsc --noEmit clean.

Checklist

  • My code adheres to this project's style guidelines
  • I have performed a self-review of my own code
  • My changes do not introduce new warnings
  • I have written tests demonstrating that my changes are effective or that my feature works
  • Local unit tests pass with my changes

The scope picker's create view fetched a single list capped at the backend's 200 per request maximum and filtered it client side via cmdk, so in deployments with more groups (issue #96 reports 1500+ from Entra SSO sync) most groups could never be found or selected. The groups section now searches server side with a debounced query and offset pagination through a new useGroupSearch hook built on the existing getGroupsFn search/limit/offset support, mirroring the GroupsTab pattern.

getAvailableScopesFn built its group name map from that same first page of 200 groups, so existing group scopes outside the window displayed their raw Mongo ObjectId instead of the group name. It now resolves names for exactly the group principalIds that have config overrides via GET /api/admin/groups/:id, fetched in small parallel batches, and falls back to the principalId when an individual group cannot be fetched.
@dustinhealy

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 95aa1ac545

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/server/groups.ts
Comment thread src/components/configuration/ScopeSelector.tsx Outdated
Comment thread src/hooks/useGroupSearch.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 95aa1ac545

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/server/groups.ts
Comment thread src/components/configuration/ScopeSelector.tsx Outdated
Comment thread src/hooks/useGroupSearch.ts Outdated
The create view filtered each backend page against existing configurations after pagination, while totalPages still described every group. A page whose groups were all configured rendered the global "all groups already have configurations" empty state even when later pages held eligible groups, and searches could report no matches for the same reason. Configured groups are now rendered as disabled entries with an "Already configured" badge instead of being filtered out, so the page contents always match the server totals and the empty states only describe truly empty result sets. Covered by ScopeSelector.test.tsx, which renders a fully configured first page and asserts disabled entries plus a reachable second page; both tests fail against the previous filtering.

Also moves the GroupSearch hook contract from useGroupSearch.ts into src/types/hooks.ts per the repository convention that locally defined interfaces live in src/types and are referenced through the @/types namespace import.
@dustinhealy

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 80c996a097

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@dustinhealy
dustinhealy marked this pull request as ready for review August 10, 2026 03:33

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 80c996a. Configure here.

Comment thread src/components/configuration/ScopeSelector.tsx
The back control in the scope create view only flipped showCreate, so useGroupSearch kept its page and filter and reopening the view rendered the previous search and page. The dialog-level resetState cleared the input through the debounced onSearchChange, so the debounced query value and page reset lagged the emptied input by the debounce delay and a reopen inside that window could still query with the stale search.

useDebouncedFilter gains a synchronous reset that cancels any pending commit and restores both values at once, useGroupSearch exposes it (clearing the page in the same call, mirrored in the GroupSearch contract in src/types/hooks.ts), and ScopeSelector routes both the back control and resetState through a shared closeCreate that resets the search state without debounce lag. The back control also gets an accessible name. Regression tests: reopening create after a search starts from page 1 unfiltered without issuing a stale search request (ScopeSelector.test.tsx), and reset clears search and page synchronously with no request for a pending debounced value (useGroupSearch.test.tsx).
@dustinhealy

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: afdb4de2ee

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/components/configuration/ScopeSelector.test.tsx
Comment thread src/components/configuration/ScopeSelector.tsx Outdated
Comment thread src/server/groups.ts
…Input

With keepPreviousData, rows from the previous page or search stayed clickable while a new group page was in flight, so a click during a slow fetch could create a configuration for a group outside the currently requested results. Group rows are now disabled while groupSearch.isFetching, alongside the existing creating and already-configured states, and the dimmed wrapper also blocks pointer events. Covered by a gated-fetch test that holds the list request open and asserts stale rows are disabled until it resolves.

The 200 character search clamp previously applied only inside getGroupsFn, so the visible input and the react-query key kept the full text while the request sent a truncated one, letting the displayed query diverge from the queried one and letting distinct long searches with a shared 200 character prefix duplicate equivalent requests under different cache keys. useGroupSearch now clamps in onSearchChange so value, debounced value, query key, and request all agree, with the server-fn clamp kept as a backstop; MAX_SEARCH_LENGTH is exported from the groups module as the single source of the limit. The truncation test now asserts the input value is clamped as well.
@dustinhealy

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 43a79ef643

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/components/configuration/ScopeSelector.tsx Outdated
Typing a new search updates the visible query immediately while the request waits out the 300ms debounce, so isFetching stayed false and rows from the previous query remained clickable against a search they no longer match. The hook now exposes isSearchPending (raw versus debounced mismatch) and the create view gates rows on it alongside isFetching.
@dustinhealy

Copy link
Copy Markdown
Contributor Author

@codex review

Comment thread src/hooks/useGroupSearch.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: af1dc214e1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/server/groups.ts
Comment thread src/components/configuration/ScopeSelector.tsx Outdated
Comment thread src/components/configuration/ScopeSelector.tsx
… GroupsTab Search

While a debounced search was fetching, keepPreviousData kept the old multi-page total visible and the paginator stayed clickable even though the rows were disabled, so clicking an old later page started the searched query at that offset and a single-page result stranded the view on an empty out-of-range page with the paginator gone. The create view now dims the paginator and ignores page changes while a fetch or debounce is pending, and useGroupSearch additionally clamps the page back to the last valid page whenever a settled total drops below it, so no code path can leave the view stranded.

useGroupSearch also discarded query errors, rendering a failed groups request as an empty list. The hook now exposes isError and refetch through the GroupSearch contract and the create view renders the existing com_error_load_groups message with a retry button in place of the list, hiding the paginator while errored.

The Groups management tab calls groupsQueryOptions directly and never goes through useGroupSearch, so its controlled input and query key kept a search beyond 200 characters while the server fn truncated it in transport. Its search handler now clamps with the shared MAX_SEARCH_LENGTH before state and debounce, keeping displayed and queried text identical, with the server-fn clamp still in place as a backstop.

Regression coverage: pagination clicks are ignored mid-search and the view settles on the searched first page, an out-of-range page clamps back to the last valid page, a failed fetch renders the retryable error state and retry recovers, and the GroupsTab input value matches the queried search at 200 characters. All four fail before this change.
@dustinhealy

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 5f032862c0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Group-based configuration breaks past 200 groups - scope picker can't show all groups and scope labels fall back to Mongo _id

1 participant